fix(idempotency): make idempotent_function decorator thread safe #1899

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merged

leandrodamascena merged 7 commits into aws-powertools:develop from mploski:fix/make-idempotency-function-thread-safe

Feb 9, 2023

Contributor

mploski commented Feb 6, 2023 •

edited by leandrodamascena

Loading

Issue number: #1718

Summary

PR modifies idempotency feature to make sure decorating user function with idempotency decorator is thread safe

Changes

Please provide a summary of what's being changed

PR modifies idempotency feature to use low level boto3 dynamodb client which is thread safe. It also adds other features/fixes as:

End2end test to ensure function decorated with idempotency features can be used in multiple parallel threads
Fix pylance type checking in an if condition
Add type to DynamoDB conditions/items in Idempotency utility and functional tests
Fix some other small stylistic errors

User experience

Please share what the user experience looks like before and after this change

User experience stays the same, but user doesn't need to worry about thread safety

Checklist

If your change doesn't seem to apply, please leave them unchecked.

Meet tenets criteria
I have performed a self-review of this change
Changes have been tested
Changes are documented
PR title follows conventional commit semantics

Is this a breaking change?

RFC issue number:

Checklist:

Migration process documented
Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

Release bot and others added 2 commits

February 5, 2023 20:35


          update changelog with latest changes

4af71c2


          Add low level dynamodb client

d687ce9

mploski requested a review from a team as a code owner

February 6, 2023 07:57

mploski requested review from leandrodamascena and removed request for a team

February 6, 2023 07:57

boring-cyborg bot added event_handlers logger tests typing labels

pull-request-size bot added the size/XXL label

mploski changed the title ~~Fix/make idempotency function thread safe~~ Fix(idempotency): Make idempotency function thread safe

mploski changed the title ~~Fix(idempotency): Make idempotency function thread safe~~ fix(idempotency): Make idempotency function thread safe

Contributor

github-actions bot commented Feb 6, 2023

⚠️Large PR detected⚠️

Please consider breaking into smaller PRs to avoid significant review delays. Ignore if this PR has naturally grown to this size after reviews.

4 similar comments

Contributor

github-actions bot commented Feb 6, 2023

⚠️Large PR detected⚠️

Please consider breaking into smaller PRs to avoid significant review delays. Ignore if this PR has naturally grown to this size after reviews.

Contributor

github-actions bot commented Feb 6, 2023

⚠️Large PR detected⚠️

Please consider breaking into smaller PRs to avoid significant review delays. Ignore if this PR has naturally grown to this size after reviews.

Contributor

github-actions bot commented Feb 6, 2023

⚠️Large PR detected⚠️

Please consider breaking into smaller PRs to avoid significant review delays. Ignore if this PR has naturally grown to this size after reviews.

Contributor

github-actions bot commented Feb 6, 2023

⚠️Large PR detected⚠️

Please consider breaking into smaller PRs to avoid significant review delays. Ignore if this PR has naturally grown to this size after reviews.

github-actions bot added the bug label


          Fix black and mypy warnings

31158dd

mploski force-pushed the fix/make-idempotency-function-thread-safe branch from c60c23e to 31158dd Compare

February 6, 2023 08:04

pull-request-size bot added size/L and removed size/XXL labels

heitorlessa linked an issue

that may be closed by this pull request

[Thread-safety] Concurrent instantiation of Boto3 clients #1718

Closed

heitorlessa requested review from heitorlessa and leandrodamascena and removed request for leandrodamascena and heitorlessa

February 6, 2023 09:03

Contributor

heitorlessa commented Feb 6, 2023

error (must be from resource client -> low-level client):

          raise NoRegionError()
E botocore.exceptions.NoRegionError: You must specify a region.

leandrodamascena requested changes

View reviewed changes

Contributor

leandrodamascena left a comment

Hello @mploski! Another great job improving the Idempotency utility! I made some comments and I think we need to address them before starting a review to merge this.

Could you please check the comments and resolve them?

Thank you very much.

aws_lambda_powertools/utilities/idempotency/persistence/dynamodb.py Show resolved Hide resolved

tests/e2e/idempotency/test_idempotency_dynamodb.py Outdated Show resolved Hide resolved


          Fix test error + add missing pytest groupings

a2744c9


          Simplify parallel function handler

437936f

codecov-commenter commented Feb 6, 2023 •

edited

Loading

Codecov Report

Base: 97.52% // Head: 97.51% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (ef61b69) compared to base (aca5ff1).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1899      +/-   ##
===========================================
- Coverage    97.52%   97.51%   -0.01%     
===========================================
  Files          143      143              
  Lines         6573     6570       -3     
  Branches       468      468              
===========================================
- Hits          6410     6407       -3     
  Misses         128      128              
  Partials        35       35

Impacted Files	Coverage Δ
aws_lambda_powertools/event_handler/api_gateway.py	`100.00% <ø> (ø)`
aws_lambda_powertools/logging/utils.py	`100.00% <ø> (ø)`
...ambda_powertools/utilities/feature_flags/schema.py	`97.19% <ø> (ø)`
...wertools/utilities/idempotency/persistence/base.py	`99.39% <ø> (ø)`
...ools/utilities/idempotency/persistence/dynamodb.py	`98.76% <100.00%> (-0.05%)`	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

mploski requested a review from leandrodamascena

February 6, 2023 22:10

Contributor Author

mploski commented Feb 6, 2023

Hey @leandrodamascena, thx for looking into this :-) I fixed last pytest error and reverted back lines related to pytest tests grouping. Please re-review if you have some time.

leandrodamascena reviewed

View reviewed changes

aws_lambda_powertools/utilities/idempotency/persistence/dynamodb.py Show resolved Hide resolved

leandrodamascena requested changes

View reviewed changes

Contributor

leandrodamascena left a comment

I just made a suggestion to make the code more readable for other people. But I tested it locally and everything is working as expected. Tomorrow I will do another review and I think we are very close to a merger.
Thanks a lot @mploski, your codes always bring me great insights.

tests/e2e/idempotency/test_idempotency_dynamodb.py Outdated Show resolved Hide resolved

mploski requested a review from leandrodamascena

February 7, 2023 12:02


          Rename e2e tests

6b9d875

heitorlessa changed the title ~~fix(idempotency): Make idempotency function thread safe~~ fix(idempotency): make idempotency function thread safe


          fix(idempotency): updating documentation

ef61b69

boring-cyborg bot added the documentation label

leandrodamascena approved these changes

View reviewed changes

Contributor

leandrodamascena left a comment •

edited

Loading

I added a slight change to the documentation. Thanks so much for the effort to make this utility even more resilient, @mploski!

If you don't have any considerations, we are ready to merge!

aws_lambda_powertools/utilities/idempotency/persistence/dynamodb.py Show resolved Hide resolved

tests/e2e/idempotency/test_idempotency_dynamodb.py Outdated Show resolved Hide resolved

Contributor Author

mploski commented Feb 8, 2023

I added a slight change to the documentation. Thanks so much for the effort to make this utility even more resilient, @mploski!

If you don't have any considerations, we are ready to merge!

@leandrodamascena thanks for reviewing it! :-) Let's ship it

heitorlessa changed the title ~~fix(idempotency): make idempotency function thread safe~~ fix(idempotency): make idempotent_function decorator thread safe

leandrodamascena merged commit cca99ba into aws-powertools:develop

Contributor

github-actions bot commented Feb 9, 2023

@awslabs/aws-lambda-powertools-python No related issues found. Please ensure 'pending-release' label is applied before releasing.

This was referenced Mar 1, 2023

Bug: Idempotency DynamoDB _put_record method overwrites static_pk_value #1968

Closed

fix(idempotency): revert dict mutation that impacted static_pk_value feature #1970

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug documentation event_handlers logger size/L tests typing